Skip to content

feat: dynamic actors#4330

Draft
NathanFlurry wants to merge 2 commits intomainfrom
feat-dynamic-actors
Draft

feat: dynamic actors#4330
NathanFlurry wants to merge 2 commits intomainfrom
feat-dynamic-actors

Conversation

@NathanFlurry
Copy link
Member

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@NathanFlurry
Copy link
Member Author

NathanFlurry commented Feb 28, 2026

This change is part of the following stack:

Change managed by git-spice.

@railway-app
Copy link

railway-app bot commented Feb 28, 2026

🚅 Deployed to the rivet-pr-4330 environment in rivet-frontend

Service Status Web Updated (UTC)
frontend-inspector 😴 Sleeping (View Logs) Web Mar 1, 2026 at 8:08 am
frontend-cloud ❌ Build Failed (View Logs) Web Mar 1, 2026 at 8:00 am
website ❌ Build Failed (View Logs) Web Mar 1, 2026 at 7:59 am
mcp-hub ✅ Success (View Logs) Web Feb 28, 2026 at 3:49 am
ladle ❌ Build Failed (View Logs) Web Feb 28, 2026 at 3:49 am

@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 28, 2026

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@4330

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@4330

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@4330

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@4330

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@4330

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@4330

@rivetkit/sqlite-vfs

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sqlite-vfs@4330

@rivetkit/traces

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/traces@4330

@rivetkit/workflow-engine

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/workflow-engine@4330

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4330

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@4330

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@4330

commit: 9ad666b

@claude
Copy link

claude bot commented Feb 28, 2026

PR Review: feat: dynamic actors

Note: This is a draft PR. Feedback is provided early to help guide implementation.

This PR introduces dynamic actors to RivetKit — actors whose source code is loaded and executed at runtime in a sandboxed isolate rather than being statically bundled. It is a large and ambitious feature (+7K/-754 lines across a new rivetkit/dynamic subpath, isolate runtime, driver integration, fixtures, an example, and docs).

The core architecture is well-conceived. Below is structured feedback organized by severity.


Required Before Merging

1. Commented-out code in example
examples/dynamic-actors/src/actors.ts contains a fully commented-out second dynamicWorkflow example with a fetch call. Remove it before merging — dead commented code in examples is confusing to readers.

2. React version mismatch in package.json
examples/dynamic-actors/package.json declares react and react-dom as ^18.2.0 but the lockfile resolves them to 19.1.0. Update the version spec to ^19 to match the lockfile resolution.

3. Document context.input behavior on wake (file-system driver)
this.#actorInitialInputs.set(actorId, input) is called in both fresh-creation and wake paths. On wake, input is whatever was passed to wakeActor, not the original creation input. If a dynamic actor loader uses context.input to select which source to load, it will receive a different value on wake than on creation — a subtle but significant semantic difference. This should be documented clearly.

4. Unintended API version bump
Both asyncapi.json and openapi.json bump from 2.1.2 to 2.1.3 with no corresponding API schema changes visible in the diff. Either document why, or revert.


Recommended Fixes

5. isStaticActorInstance duck-typing fallback is fragile
The function (in src/actor/instance/mod.ts) checks instanceof ActorInstance first, then falls back to checking for specific method names by string. This breaks if methods are renamed or if an ActorInstance is wrapped/proxied. A better approach: add a readonly kind: "static" | "dynamic" discriminant to BaseActorInstance so instance-type narrowing is explicit and rename-safe.

6. ensureRequestArrayBuffer may throw in strict mode
The shim uses Object.defineProperty on a potentially sealed/frozen Request instance. Wrap in try/catch as a defensive measure.

7. responseBodyToBinary comma-string heuristic
The heuristic that detects a Uint8Array serialized as a comma-separated numeric string (e.g., "1,2,3") is fragile — it would misinterpret a legitimately comma-separated text response body as binary data. Use an explicit encoding contract between host and isolate (e.g., always base64-encode binary bodies in the envelope) instead.

8. Silent error drop in #runnerDynamicWebSocket flush
flushPendingMessages is called via void (fire-and-forget). If forwardIncomingWebSocketMessage throws during the flush, the error is silently dropped. Errors should at minimum be logged.

9. Per-request registry lookup in file-system driver
isDynamicActor calls loadActorStateOrError + lookupInRegistry on every HTTP request and WebSocket upgrade. Cache the dynamic/static result per actor ID to avoid redundant registry lookups on high-frequency request paths.

10. Dead createWithInput call in example
examples/dynamic-actors/src/server.ts calls createWithInput for the sourceCode actor, but that actor has no onCreate handler and ignores input — it always initializes with DEFAULT_DYNAMIC_ACTOR_SOURCE. Remove the dead argument.


Minor / Nice-to-Have

11. Double #dynamicRuntimes.delete in engine driver teardown
this.#dynamicRuntimes.delete(actorId) appears twice — once in the start-error-cleanup path and once unconditionally after normal teardown. Correct, but a brief comment would clarify intent.

12. Stale temp directories on crash
DYNAMIC_RUNTIME_ROOT uses a PID-suffixed path under /tmp. On crash the directory is not cleaned up. A process-exit handler or startup sweep of stale PID directories would prevent leakage.

13. Smoke test for the example
examples/dynamic-actors has no tests. A basic smoke test (actor creation, action call, source update) would prevent silent regressions.

14. Minor indentation inconsistency in App.tsx
The increment handler has mixed 3/4-space indentation inside try/catch.


Strengths

  • The BaseActorDefinition / BaseActorInstance type hierarchy is clean and correctly widens *ContextOf<AD> constraints to the base interface.
  • Extracting HibernatableWebSocketAckState into a shared class is a good refactor — it removes duplication between the engine driver and the dynamic isolate runtime.
  • The fixture infrastructure (per-actor ESM stubs, registry-static.ts / registry-dynamic.ts, esbuild bundling in registry-loader.ts) is well-structured and supports both test modes cleanly.
  • The ActorOptions split into GlobalActorOptionsSchema + InstanceActorOptionsSchema is correct — dynamic actors only declare global options and correctly skip timeout/queue config that the sandbox enforces via CPU limits.
  • The architecture doc in docs-internal/ is clear and honestly describes the temporary node_modules compatibility layer.
  • WebSocket message buffering in #runnerDynamicWebSocket correctly handles the race between gateway message delivery and isolate WebSocket readiness.

Review updated from diff analysis on 2026-03-01.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant